First implementation of Rule-0-0-2, invariant conditions.#1003
First implementation of Rule-0-0-2, invariant conditions.#1003MichaelRFairhurst wants to merge 6 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements RULE-0-0-2 from MISRA C++ 2023, which identifies controlling expressions that are invariant (always true or always false). The implementation refactors the existing M0-1-2 query for AUTOSAR to share common logic through a parameterized module approach, while allowing each standard to define its own exceptional cases.
- Introduces a shared
InvariantCondition.qlllibrary that provides reusable logic for detecting invariant conditions via parameterized modules - Implements RULE-0-0-2 for MISRA C++ 2023 with exceptions for
while(true),constexpr if, and macro-generateddo {} while(false)loops - Refactors M0-1-2 to use the shared library while maintaining its own exception logic for breaking loops and macro-affected conditions
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| rule_packages/cpp/DeadCode4.json | Adds rule package metadata for RULE-0-0-2 with appropriate properties and query configuration |
| cpp/misra/test/rules/RULE-0-0-2/test.cpp | Provides comprehensive test cases covering compliant and non-compliant scenarios including templates, macros, and constexpr |
| cpp/misra/test/rules/RULE-0-0-2/InvariantCondition.qlref | Reference file pointing to the MISRA query implementation |
| cpp/misra/test/rules/RULE-0-0-2/InvariantCondition.expected | Expected test results with 15 non-compliant cases identified |
| cpp/misra/src/rules/RULE-0-0-2/InvariantCondition.ql | Main query implementing RULE-0-0-2 with MISRA-specific exception logic |
| cpp/common/src/codingstandards/cpp/rules/invariantcondition/InvariantCondition.qll | Shared library module providing parameterized invariant condition detection logic |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll | Updates rule metadata registry to include DeadCode4 package |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode4.qll | Auto-generated query metadata for the DeadCode4 package |
| cpp/common/src/codingstandards/cpp/Literals.qll | Adds isTrue() and isFalse() convenience predicates to BoolLiteral class |
| cpp/autosar/src/rules/M0-1-2/InfeasiblePath.ql | Refactored to use shared library while maintaining AUTOSAR-specific exception logic |
| change_notes/2025-12-17-unfeasible-statement-refactor.md | Documents the refactoring with no expected behavior change for M0-1-2 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cpp/common/src/codingstandards/cpp/rules/invariantcondition/InvariantCondition.qll
Outdated
Show resolved
Hide resolved
cpp/common/src/codingstandards/cpp/rules/invariantcondition/InvariantCondition.qll
Outdated
Show resolved
Hide resolved
cpp/common/src/codingstandards/cpp/rules/invariantcondition/InvariantCondition.qll
Outdated
Show resolved
Hide resolved
cpp/common/src/codingstandards/cpp/rules/invariantcondition/InvariantCondition.qll
Outdated
Show resolved
Hide resolved
mbaluda
left a comment
There was a problem hiding this comment.
LGTM! Just a comment on the precision
| "description": "Invariant expressions in controlling statements can indicate logic errors or redundant code.", | ||
| "kind": "problem", | ||
| "name": "Controlling expressions should not be invariant", | ||
| "precision": "very-high", |
There was a problem hiding this comment.
Should we reduce the precision to medium, considering:
- known limitations in
SimpleRangeAnalysis - the MISRA rule is stated in a general way? (e.g. it includes the case "sum of six, consecutive, non-negative integers is always an odd number")
There was a problem hiding this comment.
Good question! In this case, we're using SimpleRangeAnalysis conservatively, so we'll get more FNs than FPs, and perhaps we should go with "High"?
I reran this query against various open source repos, and they're certainly pretty noisy. The biggest issue was caused by macros, I had an "and" in the exclusion predicate that should have been an "or," and I updated that.
The other common source of issues is in templates. But I think these are TPs, because the rule implies that these cases should be replaced with constexpr if.
What do you think?
There was a problem hiding this comment.
Edit: I forgot that the rule explicitly states that macros can produce do-while-false, so we shouldn't categorically exclude macros even though they're extremely noisy.
This has me considering medium again because the macros produce so much noise, however, this strongly suggests that these alerts are from the query working as intended...
Description
Implements Rule 0-0-2, invariant conditions. Similar to autosar's M0-1-2 but with different exceptional cases.
I tried using modules as a new form of query behavior sharing here. At some point in the future, we may want the shared query generator to generate this template instead of the existing one that uses abstract classes and an unparameterized problems query.
Change request type
.ql,.qll,.qlsor unit tests)Rules with added or modified queries
RULE-0-0-2M-0-1-2Release change checklist
A change note (development_handbook.md#change-notes) is required for any pull request which modifies:
If you are only adding new rule queries, a change note is not required.
Author: Is a change note required?
🚨🚨🚨
Reviewer: Confirm that format of shared queries (not the .qll file, the
.ql file that imports it) is valid by running them within VS Code.
Reviewer: Confirm that either a change note is not required or the change note is required and has been added.
Query development review checklist
For PRs that add new queries or modify existing queries, the following checklist should be completed by both the author and reviewer:
Author
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.
Reviewer
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.